Skip to content

fix(stella-core): read every instant, wait and timeout through the Sleeper port - #6486

Merged
macanderson merged 2 commits into
mainfrom
fix/core-ambient-time
Sep 10, 2026
Merged

fix(stella-core): read every instant, wait and timeout through the Sleeper port#6486
macanderson merged 2 commits into
mainfrom
fix/core-ambient-time

Conversation

@macanderson

Copy link
Copy Markdown
Owner

What this is

The follow-up #6482 said it was leaving behind. That PR fixed the two ambient reads that were also I/O-adjacent (OS entropy, wall clock) and recorded 19 Instant::now() reads as ratchet debt. This one removes every ambient read of time from stella-core's shipping code — the 19 explicit reads, the 9 hidden ones (.elapsed() is an Instant::now() in disguise), and the 4 tokio::time::timeouts, which are the runtime's timer rather than the port — and drops tokio's time feature from the crate. The ratchet baseline is empty.

The port

retry::Sleeper is now the engine's whole time port: now() -> Instant joins sleep and jitter. One port rather than a second Clock because a double cannot answer them apart — a sleeper that suspends virtually has moved its own now, and a timeout is a sleep racing a call. That is tokio's own shape: a paused runtime puts one virtual clock behind sleep and Instant::now. ports::Clock (now_ms) stays what it was — the millisecond clock hosts hand to things that stamp records (fleet ledger, runtime stamps, the hook bus) — and is untouched.

retry::bounded(sleeper, limit, call) is tokio::time::timeout written against the port: futures_util::future::select over the call and sleeper.sleep(limit), call polled first so a ready call never loses to a ready sleep, Option rather than Result because the limit passing is an answer, not an error.

What moved, in shipping code

  • Instant::now()self.sleeper.now() / sleeper.now(): driver (deadline notice, step timing, settlement, speculation pool), dispatch, completion, rate-limit park allowance, sub-agent deadline and tick, accounted call, retry attempt timing.
  • .elapsed()sleeper.now().duration_since(started) at each of the nine sites; CancelUsageGuard carries &dyn Sleeper because a Drop has no caller to hand it now.
  • tokio::time::timeoutretry::bounded: the tool dispatch ceiling, the idle generation bound, the task-deadline bound, the accounted call's idle bound. stella-core's tokio features are ["sync"].
  • TurnState::new / TurnState::from_checkpoint / BorrowedTurn::adopt take now: Instant; bounded_generation / deadline_bounded_generation take the sleeper. from_checkpoint is public, so stella-cli's resume path and stella-serve's checkpoint test pass Instant::now() — hosts are where that read belongs.
  • The hook bus times observer dispatch off the Clock it already holds (wall milliseconds); the quarantine rule needs three consecutive overruns, so one wall-clock step cannot quarantine anything. Its test advances a hand-stepped clock instead of thread::sleep(100ms).

The guard

check-core-no-io.py's floor gains .elapsed() and any tokio::time use; the tokio feature allowlist loses time; the baseline is empty and --update refuses to add to it, so any Instant::now() now fails. Harness: 27 cases (+3: .elapsed(), a tokio timer, the time feature). AGENTS.md rule 2, the crate README and the ci.yml comment say the count is zero.

Test doubles — the part worth reading

An instant-returning Sleeper double was tolerable while timeouts bypassed the port and is a lie once they go through it: EngineConfig::default() arms a 816 s model timeout and a 15 min tool timeout, and an instant sleep makes both fire the moment a provider future waits on another task. Twelve tests failed that way on the first run. The faithful double sleeps on tokio's clock and reads now from tokio::time::Instant::now().into_std(), and the files that use it run #[tokio::test(start_paused = true)]: virtual time is free while the runtime is idle and correct when it is not. driver/tests.rs's and subagent/tests.rs's shared doubles (now named TokioSleeper) and 17 + 7 test files changed that way; hard_drop_write_back and the streaming accounted-call test got the same double. Files with their own instant NoSleep that passed were left alone.

A side effect worth stating: the stella-core lib suite went from 46 s to 2.5 s, because several tests had been paying real backoff sleeps through tokio::time::timeout that the paused clock now skips.

Witness

  • retry::tests::bounded_races_the_ports_own_sleep: with a sleeper whose sleep returns at once, bounded(1h, pending) is None immediately and bounded(1h, ready(5)) is Some(5), and the port recorded exactly one 3 600 000 ms sleep. On tokio::time::timeout this waits the real hour.
  • check-core-no-io.py on main fails on 19 Instant::now() reads, 9 .elapsed() calls, 4 tokio::time uses and the time feature; here it passes with an empty baseline.

No test was deleted.

Verified

  • cargo check -p stella-core -p stella-engine -p stella-tools -p stella-serve -p stella-cli --all-targets: clean.
  • cargo test -p stella-core: 893 lib tests + every integration test pass (2.5 s lib). cargo test -p stella-engine: pass.
  • make guards-fast: green; ./scripts/test-core-no-io.sh: 27 passed; shellcheck clean.

Closes nothing by design (closes-nothing): this is the remaining half of the audit the maintainer asked for directly.

…eeper port

The engine read the monotonic clock itself in 19 places, hid nine more
behind `.elapsed()`, and bounded four calls with `tokio::time::timeout`,
which is the runtime's timer rather than the port. `Sleeper` gains
`now() -> Instant`; every deadline is a reading from it, every elapsed
time is the difference of two, and `retry::bounded` — the port's sleep
racing a call — is the timeout. stella-core's tokio features are `sync`
alone, and `make core-no-io`'s clock-read baseline is empty.

The shared test doubles sleep on tokio's clock under `start_paused`, so
a pending provider or tool is bounded the way it is in production and a
backoff costs nothing while the runtime is idle. The lib suite went from
46 s to 2.5 s.
@macanderson macanderson added the closes-nothing Substantial change that closes no issue by design (SCR-003) label Sep 10, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @macanderson, your pull request is larger than the review limit of 150,000 diff characters

…te timeout race

rustdoc's private-intra-doc-links lint failed the doc step on the link
from `Sleeper` to `bounded`; the header sentence claiming one clock
read remained was stale as well.
@macanderson
macanderson merged commit 6ef9966 into main Sep 10, 2026
28 checks passed
@macanderson
macanderson deleted the fix/core-ambient-time branch September 10, 2026 08:07
macanderson added a commit that referenced this pull request Sep 10, 2026
… sleeper doubles

#6486 routed every instant, wait and timeout in stella-core through
retry::Sleeper and left the real sources copied: a wall clock in
stella-cli, stella-runtime and stella-serve, a Tokio sleeper in two of
them, a sleeper trait of stella-fleet's own, and the same two test doubles
in about thirty files.

stella-time now holds TokioSleeper, WallClock and MonotonicClock (which
replaces the CLI's per-construction SystemClock), and ships PausedSleeper
and NoopSleeper behind a test-util feature. The copies are deleted;
stella-fleet's monitor takes the engine's Sleeper. stella-core's own unit
tests keep one copy in src/tests.rs, because a lib's unit tests are a
second build of the lib and a dev-dependency that links the lib
implements the trait for the first. tests/one_home.rs reads the tree and
fails on any other copy. ADR 0042 records the design #6486 merged and
where the sources live.

Closes #6484
macanderson added a commit that referenced this pull request Sep 10, 2026
… sleeper doubles

#6486 routed every instant, wait and timeout in stella-core through
retry::Sleeper and left the real sources copied: a wall clock in
stella-cli, stella-runtime and stella-serve, a Tokio sleeper in two of
them, a sleeper trait of stella-fleet's own, and the same two test doubles
in about thirty files.

stella-time now holds TokioSleeper, WallClock and MonotonicClock (which
replaces the CLI's per-construction SystemClock), and ships PausedSleeper
and NoopSleeper behind a test-util feature. The copies are deleted;
stella-fleet's monitor takes the engine's Sleeper. stella-core's own unit
tests keep one copy in src/tests.rs, because a lib's unit tests are a
second build of the lib and a dev-dependency that links the lib
implements the trait for the first. tests/one_home.rs reads the tree and
fails on any other copy. ADR 0042 records the design #6486 merged and
where the sources live.

Closes #6484
macanderson added a commit that referenced this pull request Sep 10, 2026
…Sleeper and Clock ports

#6486 routed every instant, wait and timeout in stella-core through
retry::Sleeper and left the real sources copied: a wall clock in
stella-cli, stella-runtime and stella-serve, a Tokio sleeper in two of
them, and a sleeper trait of stella-fleet's own with a third.

stella-time now holds TokioSleeper, WallClock and MonotonicClock (which
replaces the CLI's per-construction SystemClock), and ships PausedSleeper
and NoopSleeper behind a test-util feature for the test sweep that
follows. The copies are deleted; stella-fleet's monitor takes the
engine's Sleeper. tests/one_home.rs reads the shipping tree and fails on
any other copy. ADR 0042 records the design #6486 merged and where the
sources live.

Refs #6484
macanderson added a commit that referenced this pull request Sep 10, 2026
…Sleeper and Clock ports

#6486 routed every instant, wait and timeout in stella-core through
retry::Sleeper and left the real sources copied: a wall clock in
stella-cli, stella-runtime and stella-serve, a Tokio sleeper in two of
them, and a sleeper trait of stella-fleet's own with a third.

stella-time now holds TokioSleeper, WallClock and MonotonicClock (which
replaces the CLI's per-construction SystemClock), and ships PausedSleeper
and NoopSleeper behind a test-util feature for the test sweep that
follows. The copies are deleted; stella-fleet's monitor takes the
engine's Sleeper. tests/one_home.rs reads the shipping tree and fails on
any other copy. ADR 0042 records the design #6486 merged and where the
sources live.

Refs #6484
macanderson added a commit that referenced this pull request Sep 11, 2026
…Sleeper and Clock ports (#6488)

## What & why

#6486 routed every instant, wait and timeout in `stella-core` through
`retry::Sleeper`. It left the other half of the same defect in place:
the real time sources are copied. A Unix-epoch wall clock lived three
times (`stella-cli` `WallClock`, `stella-runtime` `HostClock`,
`stella-serve` `WallClock`); a Tokio sleeper twice (`stella-cli`,
`stella-serve`); and `stella-fleet` kept a `Sleeper` trait of its own
with a third `TokioSleeper`. `stella-serve` may not link `stella-cli` or
`stella-runtime`, and `stella-core` may not link the Tokio timer, so the
copies had no home.

This PR gives them one, and ships the two test doubles the follow-up
sweep will move every test onto.

- **`stella-time`** holds `TokioSleeper` (the engine's real sleeper and
`now`), `WallClock` (Unix epoch, for a stamp another process reads) and
`MonotonicClock` (one origin per process, for a span compared as a
number; replaces `stella-cli`'s per-construction `SystemClock`). The
copies are deleted; `stella-fleet`'s trait is gone and its monitor takes
the engine's, re-exported under the old name. Justified under AGENTS.md
§ "When a new crate is justified" on two counts: it holds the effects
the ports keep out of `stella-core`, and it sits below `stella-serve`.
Exemplar: tokio / tokio-test.
- **`stella_time::test_util`** ships `PausedSleeper` and `NoopSleeper`
behind a `test-util` feature. Nothing takes them yet: the sweep that
retires the ~30 per-file copies in `stella-core` and `stella-engine` is
the stacked follow-up PR, kept separate so each diff stays under
Sourcery's review limit and reads as one change.
- **ADR 0042** records the design #6486 merged (why `now` sits on
`Sleeper`, why the reading is an `Instant`) and where the sources and
doubles live. #6486 closed nothing and wrote no ADR; SCR-002 asks for
one.

Refs #6484 — the issue closes with the follow-up sweep, which lands its
last two checklist items.

## The witness

- [x] This PR includes a witness test (fails on `main`, passes here)

`crates/stella-time/tests/one_home.rs` reads every shipping `.rs` under
`crates/*/src` (test directories and `tests.rs` files skipped) and fails
on any `impl … Sleeper for` outside `stella-time`, and on any `struct
WallClock | HostClock | SystemClock | MonotonicClock` outside it. The
`impl`s that stay are named with reasons: two special-shape doubles (the
retry tests' recording sleeper, the monitor's advancing sleeper) and
five inline test doubles the follow-up sweep retires; a second test
fails if a named one disappears, so the list cannot go stale. On `main`
the sleeper test fails on `stella-cli/src/runtime.rs` and
`stella-serve/src/remote.rs`, and the clock test on
`stella-cli/src/runtime.rs` (`SystemClock`, `WallClock`) and
`stella-runtime/src/wrapper/stamp.rs` (`HostClock`);
`stella-fleet/src/monitor.rs` is on the kept list for its advancing
double, so its production `TokioSleeper` was reachable only through the
trait it also deleted. Locally the time crate's 6 + 3 tests pass, and
`stella-core`, `stella-engine`, `stella-serve`, `stella-runtime`,
`stella-fleet` and `stella-cli` type-check with their tests.

## The gate

- [x] `cargo fmt --check`
- [ ] clippy over the workspace at `-D warnings` — CI; ran locally,
scoped and clean, on the six touched crates
- [ ] the workspace test suite — CI
- [x] Docs updated where behavior/flags changed (README, `--help`, doc
comments)
- [x] CLA signed
- [x] No `Closes` here by design: this PR advances #6484 and the
follow-up closes it (`closes-nothing`)

## Fix over file

- [x] Extra fixes in this PR, each its own commit:
- **The two high-severity Dependabot alerts on `main`** (`sharp`
<0.35.4, GHSA-rgj7-g3m4-5g8c; `js-yaml` <4.3.2, GHSA-2883-xcg3-v3hh) are
both transitive under `website/`, so `website/pnpm-workspace.yaml`
raises the `sharp` floor and adds a `js-yaml` one, the way that file
already handles `postcss` and `nanoid`; `js-yaml` takes a caret because
a bare floor resolves to 5.x, which fumadocs does not call.
- **`dependency-review`** re-surfaced sharp's fourteen LGPL-3.0 libvips
tuples on that bump (not a required check). They are named in
`allow-dependencies-licenses`, which is the immediate remedy issue #2532
records, with the reasoning in the workflow comment: the docs site is
private, imports no `next/image`, and ships nothing into either license
track.
- `AGENTS.md` carried two crate counts ("Twenty-nine crates", "The other
twenty-four crates") that a new crate makes wrong; both are now phrased
without a number.
- [x] Nothing was deferred beyond the stacked follow-up.

## Ground-rule check

- [x] No I/O added to `stella-core`; it is untouched except its README
- [x] No new outbound network calls
- [x] No new cross-boundary serde types

## Deleted tests

Three tests in `stella-cli/src/runtime.rs` tested the clocks that module
no longer defines, and each has a counterpart in
`stella-time/src/lib.rs`:

- `system_clock_starts_near_zero_and_advances_monotonically` →
`the_monotonic_clock_never_goes_backwards` (plus
`every_monotonic_clock_shares_one_origin`, the property the old
per-construction clock lacked)
- `default_constructs_a_fresh_clock` → gone with the constructor;
`MonotonicClock` is a unit struct
- `wall_clock_reads_epoch_milliseconds_not_a_process_origin` →
`the_wall_clock_counts_from_the_unix_epoch`

## Anything reviewers should know?

- An earlier head of this branch redesigned the engine's time as `u64`
readings of `Clock`. That design lost to #6486 on the merge order and on
the merits the ADR states (a double has to answer `now` and `sleep` from
one timeline), and it was dropped rather than rebased over the merged
one.
- Sourcery declined the earlier, combined head (208k characters of diff
against a 150k limit). That is why the test-double sweep is its own PR.
- `stella-cli`'s `runtime` module is now three re-exports and
`one_shot_budget_guard`. The `SystemClock` rename to `MonotonicClock`
reaches `fleet_cmd`, `agent/engine.rs` and two test files, and nothing
else.

## Summary by Sourcery

Centralize real time sources and shared sleeper doubles in `stella-time`
while preserving the existing time ports across all hosts.

New Features:
- Add the `stella-time` crate as the shared home for Tokio-backed
sleeping, wall-clock timestamps, process-monotonic timing, and reusable
test sleeper doubles.

Bug Fixes:
- Remove duplicated clock and sleeper implementations across host crates
and standardize fleet monitoring on the engine's `Sleeper` port.
- Raise transitive website dependency floors to address the sharp and
js-yaml security advisories.

Enhancements:
- Add a source-tree witness test that prevents production time-source
implementations from being duplicated outside `stella-time`.
- Replace per-construction monotonic clocks with a process-wide shared
origin and re-export the shared implementations through existing host
modules.

Build:
- Register `stella-time` in the workspace and add it as a dependency for
the affected host crates.

CI:
- Allow the newly surfaced sharp LGPL libvips dependency tuples in
dependency review with documented rationale.

Documentation:
- Document the shared time-source boundary in the workspace guidance,
add ADR 0042, and update the ADR index and crate documentation.

Tests:
- Add coverage for wall-clock, monotonic-clock, Tokio sleeper, and
jitter behavior, plus the `stella-time` source-layout witness tests.

Chores:
- Update workspace guidance to avoid stale crate counts.
macanderson added a commit that referenced this pull request Sep 11, 2026
…a-time

About thirty test files across stella-core, stella-engine and their
tests/ directories each wrote the same Sleeper double, a no-op one under
three names and one on tokio's paused clock, and each copy grew a now()
body in #6486. They are deleted. Integration tests and stella-engine take
PausedSleeper and NoopSleeper from stella_time::test_util through a
dev-dependency; stella-core's own unit tests keep one copy in
src/tests.rs, because a lib's unit tests are a second build of the lib
and a dev-dependency that links the lib implements the trait for the
first. stella-time's one_home witness now scans test files too.

Closes #6484
macanderson added a commit that referenced this pull request Sep 11, 2026
…a-time

About thirty test files across stella-core, stella-engine and their
tests/ directories each wrote the same Sleeper double, a no-op one under
three names and one on tokio's paused clock, and each copy grew a now()
body in #6486. They are deleted. Integration tests and stella-engine take
PausedSleeper and NoopSleeper from stella_time::test_util through a
dev-dependency; stella-core's own unit tests keep one copy in
src/tests.rs, because a lib's unit tests are a second build of the lib
and a dev-dependency that links the lib implements the trait for the
first. stella-time's one_home witness now scans test files too.

Closes #6484
macanderson added a commit that referenced this pull request Sep 11, 2026
…a-time (#6499)

## What & why

The test-double half of #6484, on top of the merged `stella-time` PR
(#6488). Re-opened: the first copy (#6491) was closed by GitHub when
#6488's branch was deleted on merge. About thirty test files across
`stella-core`, `stella-engine` and their `tests/` directories each wrote
the same `Sleeper` double — a no-op one under three names, and one on
tokio's paused clock — and each copy grew a `now()` body in #6486. Every
one of them is deleted here.

- Integration tests and `stella-engine` take `PausedSleeper` and
`NoopSleeper` from `stella_time::test_util` through a dev-dependency on
`stella-time` with `test-util` on. For `stella-core` that is a
dependency cycle, which cargo allows; it is the tokio / tokio-test
shape.
- `stella-core`'s own unit tests keep one copy, `src/tests.rs`, because
a lib's unit tests are a second build of the lib and a dev-dependency
that links the lib implements the trait for the first. The compiler
forces that copy (the first attempt at taking the crate's doubles from
`stella-time` failed on every unit test with "multiple different
versions of crate `stella_core`"). It is named `tests.rs` so `make
core-no-io` reads it as test code, which is how that guard tells test
code from shipping code.
- The `one_home` witness in `stella-time` now scans test files too. Its
kept list shrinks to the four doubles with a shape a shared one cannot
have — the retry tests' recording sleeper, the monitor's advancing
sleeper, the hanging sleeper, and `step/tests.rs`'s unpaused-time
sleeper — plus the forced copy.

Closes #6484

## The witness

- [x] This PR includes a witness test (fails on `main`, passes here)

`crates/stella-time/tests/one_home.rs` reads every `.rs` under
`crates/*/src` and `crates/*/tests`. On the base branch it fails on
twenty-three files that each define a sleeper double; here it passes,
and a second test fails if a kept one disappears. Locally:
`stella-core`'s 893 lib tests and every integration test,
`stella-engine`'s tests and the time crate's 6 + 3 tests pass.

## The gate

- [x] `cargo fmt --check`
- [ ] clippy over the workspace at `-D warnings` — CI; ran locally on
`stella-core`, `stella-engine`, `stella-time`
- [ ] the workspace test suite — CI
- [x] Docs updated where behavior/flags changed (README, `--help`, doc
comments)
- [x] CLA signed
- [x] `Closes #N` appears **both** above and as a commit trailer

## Fix over file

- [x] Extra fixes in this PR: none beyond the sweep
- [x] Nothing was deferred

## Ground-rule check

- [x] No I/O added to `stella-core`; `src/tests.rs` is `#[cfg(test)]`
and `make core-no-io` is green
- [x] No new outbound network calls
- [x] No new cross-boundary serde types

## Anything reviewers should know?

- The rename `TokioSleeper` → `PausedSleeper` in the test tree is
deliberate: the paused-clock double must not share a name with the real
`stella_time::TokioSleeper`.
- Once #6498 lands (the no-io guard reads `#[cfg(test)] mod` lines),
`src/tests.rs` could take a more descriptive name; it is named for the
guard's current file-name rule.

## Summary by Sourcery

Consolidate test sleeper doubles in `stella-time` and enforce a single
ownership location across the workspace.

Enhancements:
- Centralize the shared no-op and paused-clock sleeper doubles in
`stella-time` and update `stella-core` and `stella-engine` tests to use
them.
- Retain only the specialized sleeper doubles that require unique
behavior, including the compiler-required `stella-core` unit-test copy.
- Extend the sleeper ownership witness to scan test sources and document
the shared test utility arrangement.

Build:
- Add `stella-time` as a test-only dependency with the `test-util`
feature where shared sleeper doubles are used.

Documentation:
- Update architecture and crate documentation to describe the shared
sleeper test utilities and the `stella-core` unit-test exception.

Tests:
- Update the affected unit and integration tests to use the centralized
sleeper doubles.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closes-nothing Substantial change that closes no issue by design (SCR-003)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant